Seems like we should indeed use this as it will be faster than making a new class and so far there's no differences with POS requirements. I think we can swap it out later if the POS functionality requires changes.

Some questions that come to mind which might be worth answering: How complex will it be to revert this decision later? What are the risks of affecting the store-management section of the app (eg. introducing new bugs, etc)? Does this decision make the store-management section more complex? Does it increase maintainability (eg. more testing is needed, code is more difficult to reason about, ...)? What are reasons for change to the ProductListHandler - is it dependant on the use-case (POS vs store-management) or is it a generic component?

Just to be clear, I'm not challenging whether we should re-use or not, since I honestly don't know for this case. I'm just trying to better understand the implications. In general, I'm all up for re-using where we can, but at the same time I don't think we should re-use just because the code can be invoked from two different sections of the app and works (now) -> the reason for change should imo be the same for all the sections in which the code is used or the code probably shouldn't be re-used.

Thanks for raising these questions, @malinajirka. Let me share my POV.

How complex will it be to revert this decision later?

My reasoning is we won't need to revert this decision. We may want to write a new implementation of the ProductsDataSource interface (I don't see a clear reason for doing it now though):

interface ProductsDataSource {
    val products: Flow<List<Product>>
    suspend fun loadProducts()
    suspend fun loadMore()
}

For now, employing the existing ProductListHandler's code to implement the interface saves us time. I think the important question could be – is the ProductsDataSource designed well for our use case?

What are the risks of affecting the store-management section of the app (eg. introducing new bugs, etc)?

An optimistic assumption is that we won't need to modify ProductListHandler implementation at all, which saves us development effort now. If we have to modify it in the future though, I'd consider forking/copying this class and adjusting it for the POS-specific purpose, or implementing from scratch. That said the store management app will not be affected at all.

Does this decision make the store-management section more complex?

No

Does it increase maintainability (eg. more testing is needed, code is more difficult to reason about, ...)?

Yes, any change to ProductListHandler will be reflected in both POS and store management modes. It will require additional awareness. If we want to avoid this, we can consider forking/copying this class.

What are reasons for change to the ProductListHandler - is it dependant on the use-case (POS vs store-management) or is it a generic component?

My POV is that ProductListHandler is a generic component, mature, and with a narrow scope of responsibilities – a helper class for handling product pagination and search modes.

Let me know what you think, @malinajirka, @kidinov, @backwardstruck.

I think the important question could be – is the ProductsDataSource designed well for our use case?

To me this is the main question. I personally haven't worked much with ProductListHandler, ProductSelectorRepository and WCProductStore. Is this any good? Idon't recall any complains that products are missing or not loading or something like that, so maybe a good sign. And also there is a good sign it doesn't do 10 network requests when products screen is open in the current app =)

API there is a bit weird thoughloadFromCacheAndFetch and loadMore returns Result<Unit> which we don't use at the moment

Good point about unused returns – we don't handle errors in this PR. I think we should add some sort of Result return types to loadProducts() and loadMore() functions and handle errors in the VM (I added a task for it).

Some other considerations related to interface design and DI:

• Do we want to support swipe to refresh
• Do we need a possibility to invalidate the data
• Do we want ProductsDataSource to be a singleton? This works as a naive in-memory data cache (optimizes perfomance), but also prevents from reloading products (risk of data being stale; also a problem in the app currently)

Do we want to support swipe to refresh

I guess this is more design question, but I think yes we should

Do we need a possibility to invalidate the data

I'd invalidate that on swipe to refresh gesture after success comes from the backend

Do we want ProductsDataSource to be a singleton? This works as a naive in-memory data cache (optimizes perfomance), but also prevents from reloading products (risk of data being stale; also a problem in the app currently)

Won't see keep what we show in memory in VM anyway?

An optimistic assumption is that we won't need to modify ProductListHandler implementation at all, which saves us development effort now. If we have to modify it in the future though, I'd consider forking/copying this class and adjusting it for the POS-specific purpose, or implementing from scratch. That said the store management app will not be affected at all.

I think one more factor which might play a role here is that we might not be the developers who will need to modify the code in the future. Will the other devs realize, they should create a copy/replace/refactor the component instead of branching POS vs not-POS and therefore coupling these two modes?

Having said all that, I'm personally also inclined towards re-using it, since as you said My POV is that ProductListHandler is a generic component, mature, and with a narrow scope of responsibilities – a helper class for handling product pagination and search modes.. But in general, I think our approach to re-using needs to consider future implications and whether we are setting future-us and other-devs for a success or a failure.

🤔 I'm wondering if we could build the cart section and the product selector more independently. It seems we are assuming they'll always be on the same screen. Or perhaps the part which makes this a bit confusing for me is that onLoadMore feels irrelevant to the cart section but still is one of the parameters to WooPosCartScreen.

P.S. I realize this is just a skeleton, but 'onButtonClicked' feels like a very generic name. Could we perhaps rename it to something more specific to make the code easier to reason about?

I'd also reconsider packaging here and naming. Maybe call the screen with scaffold CartBuildScreen (and package as well) and inside of it can have 2 packages: cart and productselector?

I split the cart and product selector into separate packages (4c7d9c8). They are now parts of the Home screen and have separate VMs.

Some questions that come to mind which might be worth answering: How complex will it be to revert this decision later? What are the risks of affecting the store-management section of the app (eg. introducing new bugs, etc)? Does this decision make the store-management section more complex? Does it increase maintainability (eg. more testing is needed, code is more difficult to reason about, ...)? What are reasons for change to the ProductListHandler - is it dependant on the use-case (POS vs store-management) or is it a generic component?

Just to be clear, I'm not challenging whether we should re-use or not, since I honestly don't know for this case. I'm just trying to better understand the implications. In general, I'm all up for re-using where we can, but at the same time I don't think we should re-use just because the code can be invoked from two different sections of the app and works (now) -> the reason for change should imo be the same for all the sections in which the code is used or the code probably shouldn't be re-used.

I frankly don't see why we need an interface in this case. ProductsDataSourceImpl defines an interface with it's public methods anyway

I changed "Cart" to "Home" as the cart is only one of the panes on the home screen.